fix(completion): cross-platform shell auto-detection for Windows paths and .exe extensions#273
Conversation
…s and .exe extensions
Walkthrough
ChangesShell detection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ This PR is linked to an issue assigned to @sandman-sh — thanks! The |
zeshi-du
left a comment
There was a problem hiding this comment.
LGTM — separator/case/.exe handling in the right order, cmd.exe correctly maps to undefined (only bash/zsh/fish completions exist), 4 new Windows-path test cases. Note for the record: the earlier Windows CI failure was a vitest worker-RPC timeout flake (2001/2001 tests passed), green on rerun.
Closes #275
Summary
Fixes shell auto-detection in
testsprite completionwhen invoked without an explicit shell argument.Problem
Currently,
detectShell(env)insrc/commands/completion.tsusesshellPath.slice(shellPath.lastIndexOf('/') + 1).On Windows or cross-platform environments using Git Bash / MSYS2 / WSL:
$SHELLuses backslashes (e.g.C:\Program Files\Git\bin\bash.exe).lastIndexOf('/')returns-1, resulting in the full path stringC:\Program Files\Git\bin\bash.exewhich failsisShell().$SHELLfilenames end in.exe(e.g.bash.exe,zsh.exe,fish.exe), causingisShell("bash.exe")to returnfalse.Solution
Enhances
detectShell(env)to:/and backward\slashes (/[/\\]/)..exeextensions (.toLowerCase().replace(/\.exe$/, '')).isShell()(bash,zsh,fish).Test Coverage
Updated
src/commands/completion.test.tswith unit tests for Windows paths,.exesuffixes, mixed slashes, and uppercase names. All 2,002 tests pass.